-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
convert more things to nanoseconds #12419
convert more things to nanoseconds #12419
Conversation
Very cool, freebsd fails building for some reason but I can't see anything in the CI logs. In theory, it's supposed to support ppoll like linux so not sure what the issue would be... |
Missing |
Need to expand the |
Oh I feel stupid. I was blocking the javascript that is apparently now required to see that... |
49caae1
to
686e2ff
Compare
686e2ff
to
f09acbc
Compare
LGTM |
Download the artifacts for this pull request: |
9ada013
to
af68bb2
Compare
Also changed |
16a91f1
to
0290f23
Compare
At this point you might also cherry-pick (and change to ns) this commit for maximum memes 6883d1e Allows to request sleep with 100ns resolution. Although Windows kernel doesn't go below 0.5ms in practice, but we can defer this limitation to kernel and request high resolution from API. I know @avih didn't like the additional code, but we are already bikeshedding the timer/timestamp code so might as well. |
Sure why not. I mean linux and macos are already using nanosecond sleeps under the hood so. |
88415d3
to
434c6ca
Compare
@@ -945,7 +945,7 @@ bool wasapi_thread_init(struct ao *ao) | |||
{ | |||
struct wasapi_state *state = ao->priv; | |||
MP_DBG(ao, "Init wasapi thread\n"); | |||
int64_t retry_wait = 1; | |||
int64_t retry_wait = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really important for this PR. But maybe we could have macros for defining wait times in more readable way
like
#define MP_TIME_S(s) ((s) * 1000000000)
#define MP_TIME_MS(ms) ((ms) * 1000000)
#define MP_TIME_US(ms) ((ms) * 1000)
#define MP_TIME_NS(ns) ((ns))
then we can abstract all zeros and multiplications. and just do MP_TIME_MS(1)
, especially we do a lot MP_TIME_S(1)
conversions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amusingly, MP_SECOND_US
already exists apparently. What an awful name. But yes, having such macros sounds like a good idea.
This is weird. The caller should be responsible for converting the value if desired. Move the conversion to player/command.c instead.
In many cases, this is purely cosmetic because poll still only accepts microseconds. There's still a gain here however since pthread_cond_timedwait can take a realtime ts now. Additionally, 37d6604 changed the value added to timeout_ms in X11 and Wayland to ensure that it would never be 0 and rounded up. This was both incomplete, several other parts of the player have this same problem like drm, and not really needed. Instead the MPCLAMP is just adjusted to have a min of 1.
Originally, this was added as purely a shim for macOS. However since we want to do high resolution polling which is not neccesarily available on all platforms, making this a generic wrapper for poll functions is useful so rename it.
On linux, several platforms poll for events over a fd. This has ms accuracy, but mpv's timer is in ns now so lots of precision is lost. We can use an mp_poll wrapper to use ppoll instead which takes a timespec directly with nanosecond precision. On systems without ppoll this falls back to old poll behavior. On wayland, we don't actually use this because ppoll completely messes up the event loop for some unknown reason.
9606c3f added mp_time_ns(). Since we apparently expose the mp_time_us() to clients already, there's no reason to not also expose the new nanosecond one.
Linux and macOS already use nanosecond resolution for their sleep functions. It was just being converted from microseconds before. Since we have mp_time_ns now, go ahead and bump the precision here. The timer for windows uses some timeBeginPeriod thing which I'm not sure what it does really but whatever just convert the units to ms like they were doing before. There's really no reason to keep the mp_sleep_us helper around. A multiplication by 1000 is trivial and underlying OS clocks have nanosecond precision.
Allows higher resolution sleeps than Sleep which has milliseconds resolution. In practice Windows kernel does not really go below 0.5ms, but we don't have to limit ourselves on API side of things and do the best we can.
434c6ca
to
a016025
Compare
Considerably more risky than #12371, so I apologize in advance (it works for me though...) |
Depends on #12371. Merely using
ppoll
wrecks havoc on wayland's event loop for some reason so I omitted that. No idea why that happens (epoll weirdness?). Maybe that can be revisited later.